-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updates to hardware/firmware version reporting #254
Conversation
Again, my apologies for a large CL (number of files) -- but it ended up touching a lot of auto-generated code and test cases... not sure how to avoid that. But, it should be pretty easy to "ignore" a bunch of the files since they're derivative changes (not manually written code). |
Ok, I've extracted some of the auto-generated changes into a separate branch, so this PR now only contains the "manual" changes that might need a review |
} | ||
|
||
private void upgradeMessage(String schemaName, JsonNode jsonNode) { | ||
new MessageUpgrader(schemaName, jsonNode).upgrade(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want more robust handling of messages that don't upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the upgrade it's run through the current schema validator to make sure the upgraded message is proper, so if "didn't upgrade" means that there are still lingering fields or something it will be caught. I'm not sure what additional handling would be useful... might depend on the specifics of the "didn't upgrade" (like, missing parameter, wrong parameters, etc...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that "its run through the current schema validator" is at least consistent enough behavior for the user so I'm fine with that.
} | ||
|
||
private void upgradeMessage(String schemaName, JsonNode jsonNode) { | ||
new MessageUpgrader(schemaName, jsonNode).upgrade(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that "its run through the current schema validator" is at least consistent enough behavior for the user so I'm fine with that.
No description provided.